ci: harden the build and API-pages workflows#843
Conversation
… the lockfile build_n_push: add a per-ref concurrency group so two quick merges to main can't race the :main tag (last push wins regardless of commit order, and the server auto-pulls :main); add permissions: contents: read; validate .dockerignore and package.json changes in the PR path filter. generate_api_pages: pin Node 20 and switch npm install -> npm ci so the run can never rewrite the now-tracked package-lock.json with a divergent macOS-resolved tree; stage only src/pages/ipa/resources instead of git add -A; drop --force from the push — a force-push from this workflow would silently rewrite main and destroy any PR merged since its checkout.
…ookup buildGitDateMap now logs a warning when it emits no dates (git missing or shallow clone) instead of silently blanking every page's Updated line and the sitemap lastmod entries; document the squash-merge assumption behind the --name-only walk. Remove the unused getGitLastModified. Note in CLAUDE.md that npm run start warns under output: 'standalone'. Gen output verified byte-identical.
Rebase the single generated-files commit onto the moved branch before pushing, so a PR merged during the multi-minute run no longer rejects the push (the failure --force was presumably papering over). A genuine conflict — a concurrent edit of the generated files themselves — still fails the run loudly with main untouched. Also serialise dispatches with a concurrency group: run history shows several same-day dispatches, and overlapping runs regenerate the same files. Sandbox-tested against a bare repo: plain push rejected on race; rebase+push lands with both commits intact; true conflict exits 1 leaving the branch tip untouched.
A run queued behind another checks out the commit pinned at its dispatch time; regenerating against that stale base means the pre-push rebase replays a snapshot diff, and a file the newer spec removed can silently survive from the prior run. Fetch + reset to the branch tip before generating so the diff is computed against reality. Also note the latest-dispatched-vs-newest-tag caveat on the concurrency comment. Sandbox-proven: with the old order a removed-in-newer-spec file survives the rebase replay; with sync-first it is gone.
|
Warning Review limit reached
Next review available in: 27 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR hardens two GitHub Actions workflows with concurrency, synchronization, deterministic installs, freshness checks, and safer publishing. It also refactors git date-map handling and warnings, and clarifies standalone startup documentation. ChangesCI Workflow Updates
Git Dates Script and Docs Update
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Dispatcher
participant Runner
participant GitRemote
participant DockerRegistry
Dispatcher->>Runner: Start workflow with concurrency controls
Runner->>GitRemote: Sync or check current branch/ref
Runner->>Runner: Install dependencies and generate/build artifacts
Runner->>DockerRegistry: Push only when the ref or dispatch is current
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build_n_push.yml:
- Around line 70-95: Replace the separate freshness check and unconditional
action-level promotion in the “Check whether this ref is still current”/“Docker
build and push” flow with an atomic or conditionally coordinated final ref
validation and tag publication mechanism. Ensure an older non-PR run cannot
publish stale content after a newer commit advances the ref; do not rely solely
on a second check immediately before the existing push action.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d92fc721-3c08-4625-9f00-e3ca6792fb09
📒 Files selected for processing (2)
.github/workflows/build_n_push.yml.github/workflows/generate_api_pages.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/generate_api_pages.yml
Why
Two gaps in the pipeline around the recently-tracked lockfile and the
:mainimage tag:build_n_pushhad no concurrency group: two quick merges tomainrace the image push, and whichever finishes last wins the:maintag regardless of commit order — the server pulls:main, so an older build can silently ship.generate_api_pagesrannpm installon an unpinned Node and committed withgit add -A+git push --force. Sincepackage-lock.jsonis now tracked, a divergent macOS-resolved lockfile could be swept into the auto-commit; and a force-push from this workflow would rewritemain, destroying any PR merged during its multi-minute run.What
build_n_push.yml
permissions: contents: read..dockerignoreandpackage.json.generate_api_pages.yml
npm install→npm ci(this workflow never changes dependencies, so it installs the lockfile exactly and can never rewrite it).src/pages/ipa/resources(change-check scoped to match).--force: ifmainmoved during the run, the single generated-files commit replays cleanly on top; a genuine conflict fails the run loudly withmainuntouched. If anyone remembers a deliberate reason the push was--force, please flag it.scripts/git-dates.mjs
<lastmod>.git log --name-onlywalk; drop the unused per-file lookup.CLAUDE.md
npm run startwarns underoutput: 'standalone'(safe to ignore locally; prod runsnode server.js).Summary by CodeRabbit